Skip to content

fix(state): strip _delegate_from from gateway main sessions (#109073, salvage #109081) - #118547

Open
duhman wants to merge 3 commits into
NousResearch:mainfrom
duhman:fix/gateway-strip-delegate-from-109073
Open

duhman wants to merge 3 commits into
NousResearch:mainfrom
duhman:fix/gateway-strip-delegate-from-109073

Conversation

@duhman

@duhman duhman commented Sep 21, 2026 •

Copy link
Copy Markdown

Symptom

A main gateway session (session_key set, e.g. agent:main:telegram:dm:<user>) can end up with model_config._delegate_from. That marker excludes the row from every picker (list_sessions_rich, list_recent_sessions_bounded, /resume) while the gateway keeps routing into it by session_key — chat keeps working, but the conversation vanishes from the desktop sidebar.

Often observed together with _reset_from on the same row (contradictory pair). Opposite direction of #103789 / PR #105284 (children leaking into the sidebar).

Closes #109073.

Approach (salvage of #109081)

Takes @kyssta-exe's write-time + heal approach from #109081 and rebases it onto current main, then hardens it against the review findings:

  • A shared guard strips _delegate_from from keyed gateway rows on insert, merge, and full update_session_meta() replacement paths; empty session_key is treated like NULL.
  • _heal_polluted_gateway_delegate_markers runs idempotently on open, uses safe JSON extraction, guards removal with json_valid, and collapses an empty result to SQL NULL.
  • Delegate children (no / empty session_key) retain the marker; malformed sibling JSON is preserved and cannot abort healing of valid polluted rows.
  • Regression coverage locks caller-dict immutability, picker visibility, full-replacement behavior, malformed JSON handling, and sole-marker → SQL NULL across merge, replacement, and startup-heal paths.

Credit: original design and implementation in #109081 by @kyssta-exe. This PR adds the invariant tests and lands the same fix shape on current main so #109081 can close as superseded once this merges.

Test plan

Verified on current HEAD 671d2fed with the canonical runner:

scripts/run_tests.sh   tests/hermes_state/test_gateway_delegate_marker_strip.py   tests/hermes_state/test_bounded_recent_sessions.py   tests/hermes_state/test_delegate_child_routing_inheritance.py   tests/hermes_state/test_fts_trigram_subagent_exclusion.py
  • Focused regression file: 13 passed
  • Four-file state/session regression batch: 32 passed, 0 failed

How to verify manually

  1. Temp SessionDB: create_session(..., session_key=..., model_config={"_delegate_from": parent, "_reset_from": parent}) → row must not store _delegate_from and must appear in list_sessions_rich(min_message_count=1).
  2. Clean gateway row + patch_session_model_config(..., {"_delegate_from": parent}) → marker must not stick; row stays listable.
  3. Full replacement through update_session_meta(..., model_config_json=...) on a keyed row → marker must not stick; sole-marker replacement stores SQL NULL.
  4. Raw-SQL plant _delegate_from on a keyed row beside a malformed-JSON row, reopen SessionDB → valid row heals and returns to the list; malformed sibling and delegate child remain unchanged.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint area/sessions Session lifecycle, resume, persistence, history sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Sep 21, 2026
@duhman

duhman commented Sep 21, 2026

Copy link
Copy Markdown
Author

@teknium1 @kshitijk4poor Friendly review bump when you have a slot.

Still needed on main: a gateway row with session_key set must never keep _delegate_from (hides the DM from every picker while routing still works). Salvage of #109081 with strip-at-insert, strip-at-merge, startup heal, plus regression tests.

Happy to rebase on request. main moves fast so I stopped auto-rebasing. Focused hermes_state tests green on head c00e4072.

…arch#109073)

A main gateway session (session_key set) must never carry _delegate_from —
that marker hides the row from every picker while the gateway keeps routing
into it. Salvage of kyssta-exe NousResearch#109081 (strip at insert/merge + startup heal)
plus regression tests for insert, merge, child-keep, and heal paths.
…arch#109073)

Align insert/merge empty-session_key predicates with the heal SQL, drop the
defensive merge try/except in favour of row["session_key"], and lock the
remaining edge cases (caller-dict immutability, sole-marker → NULL, heal
leaves children alone, merge on missing row).
@cursor
cursor Bot force-pushed the fix/gateway-strip-delegate-from-109073 branch from c00e407 to 76f9cac Compare September 25, 2026 12:09
Copilot AI lite review requested due to automatic review settings September 25, 2026 12:09

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Moderate issues remain in session write paths and startup healing that can leave gateway rows polluted or unrepaired.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2 Medium severity · 1 Low severity

Open (3)
What changed in this PR

Fixes polluted gateway sessions so active conversations remain visible in session pickers.

Changes:

  • Strips _delegate_from during gateway session writes.
  • Adds startup healing for existing polluted rows.
  • Adds regression coverage for gateway and delegate-child behavior.
File Summary
tests/​hermes_state/​test_gateway_delegate_marker_strip.py Adds regression tests; two NULL-cleanup assertions need tightening.
hermes_state_sessions.py Adds write-time safeguards; additional replacement, late-binding, and upsert paths need the same invariant.
hermes_state_schema.py Adds startup healing; malformed JSON, migration ordering, and lock-retry handling require changes.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread hermes_state_schema.py Outdated
Comment thread hermes_state_sessions.py Outdated
Comment thread tests/hermes_state/test_gateway_delegate_marker_strip.py Outdated
…Research#109073)

Co-authored-by: Adrian Martén <Adrian.marten@outlook.com>
@duhman

duhman commented Sep 25, 2026

Copy link
Copy Markdown
Author

Ready for re-review on 671d2fed.

  • Addressed, replied to, and resolved all three Copilot threads: safe malformed-JSON healing, the update_session_meta() replacement path, and strict SQL NULL assertions.
  • Updated the PR description to reflect the current implementation and verification.
  • Re-ran the canonical state/session batch on this exact head: 32 passed, 0 failed (focused regression file: 13 passed).

The current GitHub Actions runs are action_required with zero jobs, and my external-contributor access cannot approve those runs or formally request reviewers. Could a maintainer please approve the workflows and re-request Copilot or human review?

duhman commented Sep 28, 2026

Copy link
Copy Markdown
Author

@teknium1 @kshitijk4poor Gentle bump before I go quiet for a bit.

Still ready for re-review on 671d2fed. Copilot threads are addressed; focused state tests green locally. CI workflows still need a maintainer approval to run.

Happy to rebase if main has moved. Otherwise this one is waiting on eyes.

@elvin-du

elvin-du commented Oct 3, 2026

Copy link
Copy Markdown

Heads-up on scope from someone who hit this bug in the wild — not asking for any action, just flagging a gap in the guard condition that may matter for whoever reviews this.

Both this PR and #109081 gate the strip on session_key being non-empty (a gateway row). That covers the gateway case. But _delegate_from also lands on rows that have no session_key at all, and those are invisible to this guard.

Repro on a real install (gateway + Feishu DM + Desktop, sessions in state.db):

select id, source, coalesce(session_key,'(null)') as session_key,
       json_extract(model_config,'$._delegate_from') as marker
from sessions where model_config like '%_delegate_from%' and coalesce(session_key,'')='';

Returns rows where source='feishu' and session_key is NULL — the marker sits on a row that is not a gateway routing peer at all, so session_key != '' never becomes true and the strip never runs. The Desktop sidebar reads these through tui_gateway/methods_session.py → list_sessions_rich() → _session_filter_where(exclude_children=True), whose predicate is _delegate_from_json('s.model_config') IS NULL — so any such row is dropped from every session list. On that install hermes sessions list returns 3 rows while the table holds 5 for the same source.

How the row gets that way (traced in gateway.log, issue #57498):

INFO gateway.run: Pinned async-delegation completion to owning session
  20260920_162727_af3640 (was 20260827_113708_903c8fbd)
  for routing key agent:main:feishu:dm:oc_93bbc...

_resolve_async_delegation_session hands the subagent's own id to switch_session → record_gateway_session_peer, which does an unconditional SET source=?, session_key=?, chat_id=?, chat_type=? (hermes_state_gateway.py:234). The subagent row ends up owning the chat's routing identity, and its pre-existing _delegate_from marker then hides the whole conversation.

Two caveats I want to be honest about, since they argue against my own report:

  1. I could not reproduce this on current main. The switch_session CAS (expected_session_id) and the _delegate_from IS NULL guards in _CHAIN_STEP_SQL appear to already block this path. The two polluted rows on my install were written by the tree running 2026-09-17..09-25. So the live-hijack mechanism may already be fixed upstream, and what I'm reporting is residual damage plus a filter gap, not a fresh exploit path.
  2. Existing rows are not repaired by either of these PRs. The strip is applied on write; the ~940 already-polluted rows on that install stay invisible either way.

If a guard is still wanted for the session_key IS NULL OR '' side, I'm happy to open a small follow-up PR scoped to exactly that and clearly marked as distinct from this one. Not looking to duplicate work — just flagging that the current predicate leaves a hole.

@elvin-du

elvin-du commented Oct 3, 2026

Copy link
Copy Markdown

Correction to my comment above — I need to retract the main claim, because it's wrong and it would send you down a duplicate-PR path that isn't needed.

I wrote that the polluted rows have no session_key so your session_key != '' guard can't catch them. That is false. Re-checking the actual rows on the install:

select id, length(coalesce(session_key,'')) as key_len, source
from sessions where source='feishu' and model_config like '%_delegate_from%';
20260920_162727_af3640   key_len=56   feishu
20260923_133207_ad1db7   key_len=92   feishu

Both rows carry a fully populated gateway session_key — they are gateway routing peers. So your heal condition (session_key IS NOT NULL AND session_key != '') matches them, and this PR does fix the case I reported. I inferred "no session_key" from the Desktop read path not going through the gateway, which said nothing about what the column actually holds; I should have queried the rows before writing it.

What survives from my comment, and what doesn't:

  • Still valid: the Desktop symptom. list_sessions_rich hides any row whose model_config.$._delegate_from is set, so a polluted main gateway session disappears from the sidebar and hermes sessions list shows fewer rows than the table holds (5 rows for source='feishu', 3 listed). That's the user-visible part of [Bug]: Main gateway session polluted with _delegate_from marker — active Telegram DM permanently invisible from every session list while gateway keeps routing it #109073 and your PR targets it correctly.
  • Still valid: the trace. _resolve_async_delegation_session → switch_session → record_gateway_session_peer unconditionally SET source/session_key/chat_id/chat_type (hermes_state_gateway.py:234), and gateway.log shows the hijack for this exact install:
    INFO gateway.run: Pinned async-delegation completion to owning session
      20260920_162727_af3640 (was 20260827_113708_903c8fbd) for routing key agent:main:feishu:dm:oc_93bbc...
    
  • Still valid: neither of you repairs rows already on disk if the heal only runs at open — worth confirming your _heal_polluted_gateway_delegate_markers is reached on the normal open path, since that's what makes existing installs recover without a version bump.
  • Retracted: the "your guard misses these rows" gap, and my offer to open a follow-up PR for it. There's no gap to close. I'll stand down.

Sorry for the noise — happy to add any of this to #109073 if it's useful, but I'll leave the decision on merging direction entirely to you and the maintainers.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

5 participants